added initialization to CRTHit data members #161
Conversation
mt82
changed the title
kjplows
left a comment
kjplows
left a comment
There was a problem hiding this comment.
Looks good to me thanks! I'll let @PetrilloAtWork decide if using flavours of 0 is better than calling on std::numeric_limits<T> here; my one comment is if any CRT variables are expected to be 0 physically (looking at ts0_ns, x_pos etc) probably std::numeric_limits::lowest() is easier to interpret as nonsense.
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
| std::vector<uint8_t> feb_id{}; ///< FEB address | ||
| std::map< uint8_t, std::vector<std::pair<int,float> > > pesmap{}; ///< Saves signal hit information (FEB, local-channel and PE) . |
There was a problem hiding this comment.
These ones are actually not needed: non-basic types have their constructor called.
| std::vector<uint8_t> feb_id{}; ///< FEB address | |
| std::map< uint8_t, std::vector<std::pair<int,float> > > pesmap{}; ///< Saves signal hit information (FEB, local-channel and PE) . | |
| std::vector<uint8_t> feb_id; ///< FEB address | |
| std::map< uint8_t, std::vector<std::pair<int,float> > > pesmap; ///< Saves signal hit information (FEB, local-channel and PE) . |
| std::string tagger{}; ///< Name of the CRT wall (in the form of strings). | ||
|
|
||
| CRTHit() {} | ||
| CRTHit() = default; |
There was a problem hiding this comment.
You can just remove this, nobody will miss it.
| CRTHit() = default; |
| float z_err{0.f}; ///< position uncertainty in z-direction (cm). | ||
|
|
||
| std::string tagger; ///< Name of the CRT wall (in the form of strings). | ||
| std::string tagger{}; ///< Name of the CRT wall (in the form of strings). |
There was a problem hiding this comment.
As above, std::string constructor will be called and ensures an empty string.
| std::string tagger{}; ///< Name of the CRT wall (in the form of strings). | |
| std::string tagger; ///< Name of the CRT wall (in the form of strings). |
PetrilloAtWork
left a comment
PetrilloAtWork
left a comment
There was a problem hiding this comment.
Stupid GitHub ate my comment.
Which boiled down to: agree with @kjplows, define static constexpr double kInvalidD = std::numeric_limits<double>::lowest(); and kInvalidF for float, use those to initialise, and use -1 as plane value.
"Stupid GitHub" was an euphemism, by the way.
Default initialization values have been introduced for all CRTHit data members. This change ensures that variables are properly initialized at object creation, reducing the risk of undefined behavior and improving code reliability.